-
Notifications
You must be signed in to change notification settings - Fork 133
Add start_update_with_start_workflow to Otel Interceptor #1150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…. Restore existing tracing test to original state
temporalio/client.py
Outdated
[temporalio.api.workflowservice.v1.StartWorkflowExecutionResponse], None | ||
] | ||
_on_start_error: Callable[[BaseException], None] | ||
headers: Mapping[str, temporalio.api.common.v1.Payload] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unlike some other SDKs, this SDK chose to break apart the input into start and update classes, and put headers in each of those. Can you just use those instead of adding a third new header field to this set of input? Yes, it may mean copying the OTel header to a second place in the OTel interceptor, but that makes more sense than to add a new header field for everyone unnecessarily that force overwrites the more specific ones.
…into both operation inputs in otel interceptor
…extra call to payload conversion
What was changed
start_update_with_start_workflow
to the OpenTelemetry interceptor.headers
field totemporalio.client.StartWorkflowUpdateWithStartInput
StartWorkflowUpdateWithStartInput
to populate headers for both operations sent intemporalio.client._ClientImpl.start_update_with_start_workflow
Closes [Bug] OTEL metadata missing for execute_update_with_start_workflow #1053
How was this tested:
A new test was added to verify that start_update_with_start_workflow produces correct spans when called on an in flight workflow and a new workflow.